Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Remove dependency on Gson #7

Merged
merged 5 commits into from
Jun 11, 2024
Merged

chore: Remove dependency on Gson #7

merged 5 commits into from
Jun 11, 2024

Conversation

felipecsl
Copy link
Member

Description

We had a duplicate JSON library dependency on both Jackson and Gson.
This change consolidates all json serialization/deserialization into Jackson, removing Gson completely.
Also made some models immutable for good measure. All tests still pass. Moved some tests from the android-sdk codebase into here since they belong here

@felipecsl felipecsl requested review from leoromanovsky, aarsilv and a team June 11, 2024 16:51
Copy link
Contributor

@aarsilv aarsilv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Approving this as its certainly one way to go. I do know one customer was interested in keeping dependencies down for the android build, so there was talk of consolidating to what Android uses, org.json vs com.fasterxml.jackson. Any particular reason for one or the other?

Comment on lines +7 to +12
private final String key;
private final boolean enabled;
private final int totalShards;
private final VariationType variationType;
private final Map<String, Variation> variations;
private final List<Allocation> allocations;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job migrating the DTOs to be immutable 💪

@felipecsl felipecsl merged commit b97a018 into main Jun 11, 2024
3 checks passed
@felipecsl felipecsl deleted the felipecsl--nuke-gson branch June 11, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants